Skip to content

refactor: move package-header to new component#2030

Open
alexdln wants to merge 2 commits intonpmx-dev:mainfrom
alexdln:refactor/move-package-header
Open

refactor: move package-header to new component#2030
alexdln wants to merge 2 commits intonpmx-dev:mainfrom
alexdln:refactor/move-package-header

Conversation

@alexdln
Copy link
Member

@alexdln alexdln commented Mar 10, 2026

🧭 Context

I'm working on the header updating to the new UI. To avoid making too big PR, I'm currently creating a separate PR to move the header into a separate component

I'll create a prs train next, but we can merge this part right away to make things easier for others and to reduce conflicts

Essentially, it's almost just a move. The only interesting part is package-header height calculations via global state, making it easier to share between components (I'm thinking of converting to CSS-variable later, but for now, this is better and closer to original approach)

@vercel
Copy link

vercel bot commented Mar 10, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Mar 10, 2026 11:15pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Mar 10, 2026 11:15pm
npmx-lunaria Ignored Ignored Mar 10, 2026 11:15pm

Request Review

@codecov
Copy link

codecov bot commented Mar 10, 2026

Codecov Report

❌ Patch coverage is 65.51724% with 30 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
app/components/Package/Header.vue 74.32% 13 Missing and 6 partials ⚠️
app/pages/package/[[org]]/[name].vue 0.00% 11 Missing ⚠️

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 10, 2026

📝 Walkthrough

Walkthrough

This pull request extracts the package page header into a new Vue 3 component at app/components/Package/Header.vue, and introduces a composable usePackageHeaderHeight to expose the header height as shared state. The package page (app/pages/package/[[org]]/[name].vue) is updated to use the new component, removing inline header DOM, clipboard copy logic, scroll-to-top handling, likes logic, and related header/footer observers. An accessibility test for PackageHeader is added to the a11y suite.

Possibly related PRs

Suggested labels

front

Suggested reviewers

  • danielroe
  • serhalp
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The PR description clearly explains the refactoring work: moving the package header into a separate component to support UI updates while reducing PR size and merge conflicts.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0b2f1f42-533d-4182-aa80-4a05674df696

📥 Commits

Reviewing files that changed from the base of the PR and between 3712560 and 0bed08a.

📒 Files selected for processing (3)
  • app/components/Package/Header.vue
  • app/composables/usePackageHeaderHeight.ts
  • app/pages/package/[[org]]/[name].vue

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
test/nuxt/a11y.spec.ts (1)

729-757: The new a11y case skips the moved action states.

With docsLink/codeLink set to null and latestVersion matching displayVersion, this never renders the header actions that were moved out of the page. A regression in those links/buttons would still pass the suite.

Suggested follow-up
   describe('PackageHeader', () => {
+    const baseProps = {
+      pkg: { name: 'vue' },
+      resolvedVersion: '3.5.0',
+      displayVersion: {
+        _id: '1234567890',
+        _npmVersion: '3.5.0',
+        name: 'vue',
+        version: '3.5.0',
+        dist: {
+          shasum: '1234567890',
+          signatures: [],
+          tarball: 'https://npmx.dev/package/vue/tarball',
+        },
+      },
+      latestVersion: { version: '3.5.0', tags: [] },
+      provenanceData: null,
+      provenanceStatus: 'idle',
+      docsLink: null,
+      codeLink: null,
+      isBinaryOnly: false,
+    }
+
     it('should have no accessibility violations', async () => {
       const component = await mountSuspended(PackageHeader, {
-        props: {
-          pkg: { name: 'vue' },
-          resolvedVersion: '3.5.0',
-          displayVersion: {
-            _id: '1234567890',
-            _npmVersion: '3.5.0',
-            name: 'vue',
-            version: '3.5.0',
-            dist: {
-              shasum: '1234567890',
-              signatures: [],
-              tarball: 'https://npmx.dev/package/vue/tarball',
-            },
-          },
-          latestVersion: { version: '3.5.0', tags: [] },
-          provenanceData: null,
-          provenanceStatus: 'idle',
-          docsLink: null,
-          codeLink: null,
-          isBinaryOnly: false,
-        },
+        props: baseProps,
       })
       const results = await runAxe(component)
       expect(results.violations).toEqual([])
     })
+
+    it('should have no accessibility violations with header actions visible', async () => {
+      const component = await mountSuspended(PackageHeader, {
+        props: {
+          ...baseProps,
+          latestVersion: { version: '3.6.0', tags: ['latest'] },
+          docsLink: { name: 'docs', params: { path: ['vue', 'v', '3.5.0'] } },
+          codeLink: {
+            name: 'code',
+            params: { packageName: 'vue', version: '3.5.0', filePath: '' },
+          },
+        },
+      })
+      const results = await runAxe(component)
+      expect(results.violations).toEqual([])
+    })
   })

As per coding guidelines, **/*.{test,spec}.{ts,tsx}: Write unit tests for core functionality using vitest.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6512c139-fb07-4673-97ec-6fffbd7b5d60

📥 Commits

Reviewing files that changed from the base of the PR and between 0bed08a and 06b5b05.

📒 Files selected for processing (2)
  • app/pages/package/[[org]]/[name].vue
  • test/nuxt/a11y.spec.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant